fix: correct PHPUnit mock setups in tests#1264
Conversation
Co-authored-by: garak <179866+garak@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot there's no change on phpunit version here: composer.json is still the same, and phpunit.xml.dist also. |
Correct —
The PR description I wrote was misleading; I was describing the full scope of the stacked PR, not just my additions. |
There was a problem hiding this comment.
Pull request overview
Updates the PHPUnit test suite to align mock expectations with how the use case classes actually interact with App\Badge\Model\Package after the PHPUnit upgrade, eliminating failures caused by mocking the wrong delegation path.
Changes:
- Fixes
CreateComposerLockBadgeTestandCreateCircleCiBadgeTestto mockPackage::getRepository()directly (instead of expectinggetOriginalObject()to be invoked). - Updates
SnippetGeneratorTest’s routergenerate()callback signature to accept the 3rd argument used bySnippetGenerator::generateImg(). - Makes the router mock in
SnippetGeneratorTestfail fast on unexpected route names to avoid silently masking regressions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/Service/SnippetGeneratorTest.php | Aligns router mock callback signature with production calls and makes unexpected route names explicit failures. |
| tests/Badge/Model/UseCase/CreateComposerLockBadgeTest.php | Corrects mock setup to match direct getRepository() usage in the use case. |
| tests/Badge/Model/UseCase/CreateCircleCiBadgeTest.php | Corrects mock setup to match direct getRepository() usage and ensures error-path test triggers the intended catch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fix: correct PHPUnit mock setups in tests
fix: correct PHPUnit mock setups in tests
fix: correct PHPUnit mock setups in tests
Fixes test failures caused by incorrect mock setups in the test suite.
Improvement
Summary
Fixes 4 errors and 1 failure in the PHPUnit test suite that were caused by incorrect mock setups. The
composer.jsonandphpunit.xml.distchanges (PHPUnit ^12.4, Symfony 7.3,BypassFinalExtension) are in the basephpunit-upgradebranch; this PR only fixes the resulting test failures.Root cause: Several tests mocked
Package::getOriginalObject()expecting it to be called, but the use case classes call$package->getRepository()directly on the mock. PHPUnit mocks replace all methods, so the real delegation chain is never invoked.Test fixes
CreateCircleCiBadgeTest::testShouldCreateDefaultBadgeOnError— removed$repointermediate mock andgetOriginalObjectexpectation; throwRuntimeExceptionfromgetRepository()to triggercatch (\Throwable)CreateComposerLockBadgeTest::testShouldCreateComposerLockBadge— mockgetRepository()directly instead of routing throughgetOriginalObject()CreateComposerLockBadgeTest::testShouldCreateDefaultBadgeOnError— same pattern;catch (\Exception)catches theRuntimeExceptionfromgetRepository()SnippetGeneratorTest improvements
int $referenceType = RouterInterface::ABSOLUTE_URLto the router mock callback to match the 3-argument signature used bySnippetGenerator::generateImg()?? 'repo_url'fallback with an explicit\InvalidArgumentExceptionfor unexpected route names, preventing regressions from masking unknown routes🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.